-
Notifications
You must be signed in to change notification settings - Fork 21
Add sdlstdinc math functions #120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add sdlstdinc math functions #120
Conversation
| *) | ||
| function SDL_nlog(x: cdouble): cdouble; cdecl; | ||
| external SDL_LibName | ||
| name {$IF DEFINED(DELPHI) AND DEFINED(MACOS)} '_SDL_log' {$ELSE} 'SDL_log' {$ENDIF}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is called SDL_log in SDL code. However, since identifiers are case-insensitive in Pascal, this causes a name conflict with SDL_Log, the logging function.
I have opted to keep the logging function as-is and instead rename this function to SDL_nlog ("natural logarithm"). Another naming choice could be SDL_loge ("base e logarithm"), similar to SDL_log10 ("base-10 logarithm"). Any other ideas welcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see and guess, your solution is good. It is important to extent the function comment about this. Something like:
SDL2-for-Pascal: ATTENTION: The original C name of this function is SDL_log, but since Pascal names are case-insensitive, it is in conflict with SDL_Log (logging function). Hence we needed to rename it.
Why would you use
external SDL_LibName name {$IF DEFINED(DELPHI) AND DEFINED(MACOS)} '_SDL_log' {$ELSE} 'SDL_log' {$ENDIF};
instead of
external SDL_LibName {$IFDEF DELPHI} {$IFDEF MACOS} name '_SDL_log' {$ENDIF} {$ENDIF}; ?
Same question for SDL_nlogf.
Best regards
Matthias
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a warning/note about the renaming is a good idea. Regarding the external syntax - the second form would try to link to the SDL_nlog function in the SDL library, which does not exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sure! - Was confused because you used this also for SDL_logf but you changed its C name, too, for consistency, I guess. So, I'd suggest to extend SDL_nlogf with the same/a similar hint in the comment.
Best regards,
Matthias
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the comments as discussed and merged. Thanks for this PR!
This patch adds the cstdlib-like math functions found in
SDL_stdinc.h. The doc-comments are based on Linux man-pages and research into SDL's git history.